feature-power tag for notes grid with specific tagname #3204
Conversation
ghost
assigned 
Generated by 🚫 Danger |
| @@ -79,6 +79,7 @@ def locale_name_pairs | |||
| end | |||
|
|
|||
| def insert_extras(body) | |||
| body = NodeShared.notes_inline_grid(body) | |||
There was a problem hiding this comment.
Wow, this looks great... but I think there should be a lot of overlap with the notes_grid method... do you think we could consolidate those two? In theory it should be relatively close ActiveRecord queries, but displayed differently, don't you think?
There was a problem hiding this comment.
Yes, makes sense!! I see the methods notes_grid and notes_inline_grid differ only in rendering the templates. Both render different templates having same active-record queries. So, let's keep a single function with a parameter (say thumbnails=false by default) and change the template accordingly?
|
that sounds perfect!
…On Mon, Aug 6, 2018 at 12:13 PM Vidit ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In app/helpers/application_helper.rb
<#3204 (comment)>:
> @@ -79,6 +79,7 @@ def locale_name_pairs
end
def insert_extras(body)
+ body = NodeShared.notes_inline_grid(body)
Yes, makes sense!! I see the methods notes_grid and notes_inline_grid
differ only in rendering the templates. Both render different templates
having same active-record queries. So, let's keep a single function with a
parameter (say thumbnails=false by default) and change the template
accordingly?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3204 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABfJygH6xFcRPjnQOwpA6q8iSWlK0Gpks5uOGsjgaJpZM4VvdCd>
.
|
|
@ViditChitkara want to give this a push? Thanks! |
|
Actually, the issue is solved in #3970. So, closing this PR. Thanks! |
closes #1097